Metadata V16 (unstable): Enrich metadata with associated types of config traits #5274

lexnv · 2024-08-07T17:31:24Z

This feature is part of the upcoming metadata V16. The associated types of the Config trait that require the TypeInfo or Parameter bounds are included in the metadata of the pallet. The metadata is not yet exposed to the end-user, however the metadata intermediate representation (IR) contains these types.

Developers can opt out of metadata collection of the associated types by specifying without_metadata optional attribute to the #[pallet::config].

Furthermore, the without_metadata argument can be used in combination with the newly added #[pallet::include_metadata] attribute to selectively include only certain associated types in the metadata collection.

API Design

There is nothing to collect from the associated types:

#[pallet::config]
pub trait Config: frame_system::Config {
		// Runtime events already propagated to the metadata.
		type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;

		// Constants are already propagated.
		#[pallet::constant]
		type MyGetParam2: Get<u32>;
	}

Default automatic collection of associated types that require TypeInfo or Parameter bounds:

	#[pallet::config]
	pub trait Config: frame_system::Config {
		// Runtime events already propagated to the metadata.
		type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;

		// Constants are already propagated.
		#[pallet::constant]
		type MyGetParam2: Get<u32>;

		// Associated type included by default, because it requires TypeInfo bound.
		/// Nonce doc.
		type Nonce: TypeInfo;

		// Associated type included by default, because it requires
		// Parameter bound (indirect TypeInfo).
		type AccountData: Parameter;

		// Associated type without metadata bounds, not included.
		type NotIncluded: From<u8>;
	}

Disable automatic collection

// Associated types are not collected by default.
	#[pallet::config(without_metadata)]
	pub trait Config: frame_system::Config {
		// Runtime events already propagated to the metadata.
		type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;

		// Constants are already propagated.
		#[pallet::constant]
		type MyGetParam2: Get<u32>;

		// Explicitly include associated types.
		#[pallet::include_metadata]
		type Nonce: TypeInfo;

		type AccountData: Parameter;

		type NotIncluded: From<u8>;
	}

Builds on top of the PoC: #4358
Closes: #4519

cc @paritytech/subxt-team

Signed-off-by: Alexandru Vasile <[email protected]>

types Signed-off-by: Alexandru Vasile <[email protected]>

Signed-off-by: Alexandru Vasile <[email protected]>

…associated-types

Signed-off-by: Alexandru Vasile <[email protected]>

Co-authored-by: Guillaume Thiolliere <[email protected]>

Signed-off-by: Alexandru Vasile <[email protected]>

gui1117 · 2024-10-10T16:41:44Z

substrate/frame/support/test/tests/pallet_ui/config_metadata_non_type_info.stderr

+error[E0277]: the trait bound `<T as pallet::Config>::MyNonScaleTypeInfo: TypeInfo` is not satisfied
+  --> tests/pallet_ui/config_metadata_non_type_info.rs:29:8
+   |
+29 |         type MyNonScaleTypeInfo;
+   |              ^^^^^^^^^^^^^^^^^^ the trait `TypeInfo` is not implemented for `<T as pallet::Config>::MyNonScaleTypeInfo`
+   |


In general I prefer that error originating from expansion from an attribute point directly to the attribute.

Like here it would point to #[pallet::include_metadata], instead of the associated type ident.

This allows user to directly look at the doc of include_metadata and not be confused trying to understand it as a regular rust error.

But in this case I don't know, it is also fine as it is.

I see what you mean now! Indeed, have added an extra check while collecting the metadata, we should have now a bit clearer error message here:

error: Invalid #[pallet::include_metadata] in #[pallet::config], collected type `MyNonScaleTypeInfo` does not implement `scale::TypeInfo` --> tests/pallet_ui/config_metadata_non_type_info.rs:28:4

actually what I imagine was something like:

let span = /* span of `#[pallet::include_metadata]" or otherwise call site for automatic inclusion*/ quote_spanned(span => /* expansion related to include metadata or automatic inclusion*/)

But everything is good to me, I don't have much opinion.
Your current implementation is more restrictive but more explicit so it can be the best.

gui1117 · 2024-10-10T16:47:31Z

Looks good to me, but zombienet tests are failing somehow.

Signed-off-by: Alexandru Vasile <[email protected]>

lexnv · 2024-10-11T12:42:41Z

@bkchr could you please have a quick look whenever you have some time? 🙏

Thanks everyone for the reviews here 🙏

gui1117 · 2024-10-12T16:30:22Z

substrate/frame/support/procedural/src/pallet/parse/config.rs

+					if !contains_type_info_bound(ty) {
+						let msg = format!(
+						"Invalid #[pallet::include_metadata] in #[pallet::config], collected type `{}` \
+						does not implement `scale::TypeInfo`",


nitpick, maybe we can say: does not explicitly bound TypeInfo or Parameter.

gui1117 · 2024-10-12T16:33:43Z

substrate/frame/support/procedural/src/pallet/parse/config.rs

+	ty.bounds.iter().any(|bound| {
+		let syn::TypeParamBound::Trait(bound) = bound else { return false };
+
+		KNOWN_TYPE_INFO_BOUNDS.iter().any(|known| bound.path.is_ident(known))


I think that this will not match scale::TypeInfo but only TypeInfo, isn't it?

What I mean is that we do the match on the whole path, not the last segment, no?

If we want to support only this. Then we can say in the error message of include_metadata something like: "Note: only ident is supported, e.g. scale::TypeInfo is not detected, only TypeInfo.

We should just take the last element of the path to improve the detection?

bkchr

Mainly left some nitpicks. Generally looking good. Good work 👍

bkchr · 2024-10-14T15:04:18Z

substrate/frame/support/procedural/src/pallet/parse/config.rs

+				}
+
+				if let syn::TraitItem::Type(ref ty) = trait_item {
+					if !contains_type_info_bound(ty) {


Could we implement here a mix of:

https://docs.rs/static_assertions/1.1.0/src/static_assertions/assert_impl.rs.html#329-356

and

https://doc.rust-lang.org/beta/reference/attributes/diagnostics.html#the-diagnosticon_unimplemented-attribute

This should give some very nice error message.

Ahh but maybe not possible. Not sure, but worth a try.

bkchr · 2024-10-14T15:10:28Z

substrate/frame/support/procedural/src/pallet/parse/config.rs

+					//
+					// They must provide a type item that implements `TypeInfo`.
+					(PalletAttrType::IncludeMetadata(_), syn::TraitItem::Type(ref typ)) => {
+						already_collected_associated_type = Some(pallet_attr._bracket.span.join());


There should be an error if the attribute is passed multiple times.

bkchr · 2024-10-14T15:12:44Z

substrate/frame/support/procedural/src/pallet/parse/config.rs

+	ty.bounds.iter().any(|bound| {
+		let syn::TypeParamBound::Trait(bound) = bound else { return false };
+
+		KNOWN_TYPE_INFO_BOUNDS.iter().any(|known| bound.path.is_ident(known))


We should just take the last element of the path to improve the detection?

bkchr · 2024-10-14T15:50:42Z

substrate/frame/support/procedural/src/pallet/parse/mod.rs

+				let fields: syn::punctuated::Punctuated<ConfigValue, syn::Token![,]> =
+					inside_config.parse_terminated(ConfigValue::parse, syn::Token![,])?;
+				let config_values = fields.iter().collect::<Vec<_>>();
+
+				let frequencies = config_values.iter().fold(HashMap::new(), |mut map, val| {
+					let string_name = match val {
+						ConfigValue::WithDefault(_) => "with_default",
+						ConfigValue::WithoutAutomaticMetadata(_) => "without_automatic_metadata",
+					}
+					.to_string();
+					map.entry(string_name).and_modify(|frq| *frq += 1).or_insert(1);
+					map
+				});
+				let with_default = frequencies.get("with_default").copied().unwrap_or(0) > 0;
+				let without_automatic_metadata =
+					frequencies.get("without_automatic_metadata").copied().unwrap_or(0) > 0;
+
+				let duplicates = frequencies
+					.into_iter()
+					.filter_map(|(name, frq)| if frq > 1 { Some(name) } else { None })
+					.collect::<Vec<_>>();
+				if !duplicates.is_empty() {
+					let msg = format!("Invalid duplicated attribute for `#[pallet::config]`. Please remove duplicates: {}.", duplicates.join(", "));
+					return Err(syn::Error::new(span, msg));
+				}


Maybe just having some options would be easier here? But yeah, the code works the way it is written.

…associated-types

Signed-off-by: Alexandru Vasile <[email protected]>

This PR exposes the *unstable* metadata V16. The metadata is exposed under the unstable u32::MAX number. Developers can start experimenting with the new features of the metadata v16. *Please note that this metadata is under development and expect breaking changes until stabilization.* The `ExtrinsicMetadata` trait receives a breaking change. Its associated type `VERSION` is rename to `VERSIONS` and now supports a constant static list of metadata versions. The versions implemented for `UncheckedExtrinsic` are v4 (legacy version) and v5 (new version). For metadata collection, it is assumed that all `TransactionExtensions` are under version 0. Builds on top of: #5274 Closes: #5980 Closes: #5347 Closes: #5285 cc @paritytech/subxt-team --------- Signed-off-by: Alexandru Vasile <[email protected]> Co-authored-by: Niklas Adolfsson <[email protected]> Co-authored-by: Bastian Köcher <[email protected]> Co-authored-by: James Wilson <[email protected]> Co-authored-by: GitHub Action <[email protected]>

lexnv added 9 commits August 7, 2024 18:05

metadata-ir: Introduce PalletAssociatedTypeMetadata

7e8baee

Signed-off-by: Alexandru Vasile <[email protected]>

frame/config: Add associated types to parsed config

1c12f15

Signed-off-by: Alexandru Vasile <[email protected]>

frame/expand: Implement associated types expansion

20b22d8

Signed-off-by: Alexandru Vasile <[email protected]>

frame/expand: Use provided cfgs for the associated types

350efce

Signed-off-by: Alexandru Vasile <[email protected]>

frame/construct_runtime: Extract associated types from pallet config

3e97971

Signed-off-by: Alexandru Vasile <[email protected]>

frame/pallet: Introduce config(without_metadata)

5d8021e

Signed-off-by: Alexandru Vasile <[email protected]>

frame/pallet: Introduce #[pallet::include_metadata] for associated

5679244

types Signed-off-by: Alexandru Vasile <[email protected]>

frame/pallet: Include associated type iff bounds contain TypeInfo

958611e

Signed-off-by: Alexandru Vasile <[email protected]>

frame/pallet: Proper flag for metdata collection

7f26b67

Signed-off-by: Alexandru Vasile <[email protected]>

lexnv self-assigned this Aug 7, 2024

lexnv mentioned this pull request Aug 7, 2024

wip/PoC: Enrich metadata custom types with essential types for chain communications #4358

Closed

lexnv added 9 commits August 8, 2024 11:05

frame/tests/ui: Fix type in test

cc001f3

Signed-off-by: Alexandru Vasile <[email protected]>

frame/tests/ui: Check config without metadata

b2803e2

Signed-off-by: Alexandru Vasile <[email protected]>

frame/tests/ui: Check config with multiple attributes

94007d8

Signed-off-by: Alexandru Vasile <[email protected]>

frame/tests/ui: Add negative test for duplicate attributes in config

75d2697

Signed-off-by: Alexandru Vasile <[email protected]>

frame/tests/ui: Add negative test for collecting metadata from constants

ebcb4a0

Signed-off-by: Alexandru Vasile <[email protected]>

frame/tests/ui: Add negative test for metadata collection on events

5115021

Signed-off-by: Alexandru Vasile <[email protected]>

frame/tests: Check PalletAssociatedTypeMetadataIR collection

aba029a

Signed-off-by: Alexandru Vasile <[email protected]>

frame/support: Add documentation

6502c83

Signed-off-by: Alexandru Vasile <[email protected]>

Add PRdoc

e9571cb

Signed-off-by: Alexandru Vasile <[email protected]>

lexnv added I5-enhancement An additional feature request. T1-FRAME This PR/Issue is related to core FRAME, the framework. T11-documentation This PR/Issue is related to documentation. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. labels Aug 8, 2024

lexnv marked this pull request as ready for review August 8, 2024 10:36

lexnv requested a review from a team as a code owner August 8, 2024 10:36

lexnv added 4 commits August 8, 2024 13:41

Merge remote-tracking branch 'origin/master' into lexnv/metadata-v16-…

31bf284

…associated-types

Update prdoc

7429eb7

Signed-off-by: Alexandru Vasile <[email protected]>

prdoc: Remove unneeded crate

8a0c138

Signed-off-by: Alexandru Vasile <[email protected]>

prdoc: Include frame-support

c6c1800

Signed-off-by: Alexandru Vasile <[email protected]>

lexnv and others added 2 commits October 10, 2024 08:35

Update substrate/frame/support/procedural/src/pallet/parse/config.rs

cf6749b

Co-authored-by: Guillaume Thiolliere <[email protected]>

Update substrate/frame/support/procedural/src/pallet/parse/config.rs

54e6d2a

Co-authored-by: Guillaume Thiolliere <[email protected]>

paritytech-review-bot bot requested a review from a team October 10, 2024 05:36

lexnv added 6 commits October 10, 2024 08:38

frame/pallet: Adjust error messages

610bdc2

Signed-off-by: Alexandru Vasile <[email protected]>

pallet/expand: Use associated type span for better errors

b18a2f9

Signed-off-by: Alexandru Vasile <[email protected]>

pallet/parse: Use lookahead properly

678944a

Signed-off-by: Alexandru Vasile <[email protected]>

tests/pallet_ui: Add include metadata for nonscale type info

d43743c

Signed-off-by: Alexandru Vasile <[email protected]>

frame/support: Rename #[without_metadata] ->

0928d78

Signed-off-by: Alexandru Vasile <[email protected]>

frame/support: Rename WithoutAutomaticMetadata variant

8fd0180

Signed-off-by: Alexandru Vasile <[email protected]>

gui1117 approved these changes Oct 10, 2024

View reviewed changes

lexnv and others added 3 commits October 11, 2024 12:30

frame/support: Better error msg for invalid #[include_metadata]

ac08308

Signed-off-by: Alexandru Vasile <[email protected]>

pallet_ui/tests: Update error message

1296dc2

Signed-off-by: Alexandru Vasile <[email protected]>

Merge branch 'master' into lexnv/metadata-v16-associated-types

c1f204f

gui1117 reviewed Oct 12, 2024

View reviewed changes

bkchr approved these changes Oct 14, 2024

View reviewed changes

lexnv and others added 6 commits October 15, 2024 16:32

Merge remote-tracking branch 'origin/master' into lexnv/metadata-v16-…

bc0f528

…associated-types

frame/support: Detect #[include_metadata] duplicates

b125a1e

Signed-off-by: Alexandru Vasile <[email protected]>

frame/support: Improve TypeInfo detection

39a9d4b

Signed-off-by: Alexandru Vasile <[email protected]>

frame/support: Simplify duplicate detection logic

ac92bc6

Signed-off-by: Alexandru Vasile <[email protected]>

frame/support: Rename error for explicit types

e96b36d

Signed-off-by: Alexandru Vasile <[email protected]>

Merge branch 'master' into lexnv/metadata-v16-associated-types

e91da23

lexnv enabled auto-merge October 15, 2024 16:09

Merge branch 'master' into lexnv/metadata-v16-associated-types

f3e3e1e

lexnv added this pull request to the merge queue Oct 16, 2024

Merged via the queue into master with commit b649f4a Oct 16, 2024
189 of 193 checks passed

lexnv deleted the lexnv/metadata-v16-associated-types branch October 16, 2024 10:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Metadata V16 (unstable): Enrich metadata with associated types of config traits #5274

Metadata V16 (unstable): Enrich metadata with associated types of config traits #5274

lexnv commented Aug 7, 2024 •

edited

Loading

gui1117 Oct 10, 2024

lexnv Oct 11, 2024

gui1117 Oct 12, 2024

gui1117 commented Oct 10, 2024

lexnv commented Oct 11, 2024

gui1117 Oct 12, 2024

gui1117 Oct 12, 2024

bkchr Oct 14, 2024

bkchr left a comment

bkchr Oct 14, 2024

bkchr Oct 14, 2024

bkchr Oct 14, 2024

bkchr Oct 14, 2024

bkchr Oct 14, 2024

Metadata V16 (unstable): Enrich metadata with associated types of config traits #5274

Metadata V16 (unstable): Enrich metadata with associated types of config traits #5274

Conversation

lexnv commented Aug 7, 2024 • edited Loading

API Design

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gui1117 commented Oct 10, 2024

lexnv commented Oct 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkchr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lexnv commented Aug 7, 2024 •

edited

Loading